Skip to content

feat: Github PR creation #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

feat: Github PR creation #30

wants to merge 27 commits into from

Conversation

Xaroz
Copy link
Contributor

@Xaroz Xaroz commented Jun 23, 2025

This PR allows the deploy app to offer auto creation of pull request to the registry using octokit package as a base.
Fixes ENG-1671

  • Creates handler for an API service where users can submit their deploy configs
  • The service is fully validated before being able to submit
  • Creates Modal for users to accept submitting a PR

TODO:

  • Add changeset
  • Run eslint rules properly on configs
  • Add validation/auth to avoid repeated requests/API abuse

For the Eslint rules instead of using ESLint which is not allowed in the browser, I sorted the tokens by chain name and the connections by their chain name too, which would match the rules we have in the registry plus yaml takes care of sorting the config by their keys

Summary by CodeRabbit

  • New Features

    • Added ability to create a GitHub pull request to add a Warp route deployment to the Hyperlane Registry directly from the deployment success screen.
    • Introduced new modals and UI elements for submitting and confirming registry PRs.
    • Added server-side API endpoint for securely creating deployment PRs, including cryptographic signature verification.
    • Enhanced configuration validation and management for server environment variables.
    • Provided utility functions for sorting configuration files and normalizing data.
    • Added React hook to facilitate PR creation with cryptographic signing.
    • Included server-side GitHub client caching for improved API interaction efficiency.
  • Bug Fixes

    • Improved error handling in toast notifications to better display nested error messages.
  • Style

    • Applied formatting and stylistic improvements to configuration files for consistency.
  • Chores

    • Added new dependencies for GitHub API and human-readable IDs.
    • Refactored and extended utility modules for API responses, object sorting, and string normalization.

Copy link

vercel bot commented Jun 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hyperlane-deploy-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 14, 2025 4:29pm

@Xaroz Xaroz marked this pull request as ready for review June 24, 2025 22:38
@Xaroz Xaroz requested a review from cmcewen as a code owner June 24, 2025 22:38
fixes
[ENG-1824](https://linear.app/hyperlane-xyz/issue/ENG-1824/rate-limiting-avoid-spamming-of-endpoints)

This PR features different ways to prevent the `create-pr` endpoint from
being spammed, the process goes like:

1. In the UI, the temporary deployer wallet will sign a message that
contains a `timestamp` and `warpRouteId`.
2. The returned `signature` will be sent to the endpoint along with the
`address` and `message`
3. The backend service will verify the signature using viem's
`verifySignature` and also check if the `timestamp` has expired (its
currently set to 2 minutes)
4. A `keccak256` hash will be created using the `warpRouteId`,
`deployConfig` and `warpConfig`, this will be used for the branch name,
which will be in `warpRouteId-keccak256hash` format
5. This branch name will be verified using `octokit` so no other
branches exists with the same name
6. If every check pass then the PR is created

Rate limiting will be implemented at a later PR

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added a clear message in the PR creation modal informing users that
wallet signature is required for verification, with no on-chain
transaction involved.
* Introduced wallet-based signature verification for PR creation,
enhancing security and authenticity.
* Implemented deterministic branch naming to prevent duplicate PRs for
the same configuration.

* **Bug Fixes**
* Prevented duplicate pull requests by checking for existing branches
with the same configuration.

* **Refactor**
* Improved internal structure for PR creation and signature verification
processes to enhance maintainability.

* **Chores**
* Added utility for sorting object keys to ensure consistent
configuration handling.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Copy link

coderabbitai bot commented Jul 8, 2025

📝 Walkthrough

Walkthrough

This update introduces a full workflow for creating GitHub pull requests to add Warp route deployments to the Hyperlane Registry. It includes new API endpoints, server/client utilities, React components, and validation logic. Supporting utilities for config sorting, error handling, and string normalization are also added, with several UI and code structure enhancements.

Changes

Files/Paths Change Summary
src/features/deployment/github.ts, src/libs/github.ts, src/pages/api/create-pr.ts New modules for GitHub PR creation, Octokit client caching, and API route for PR submission, including signature verification.
src/features/deployment/CreateRegistryPrModal.tsx, src/features/deployment/warp/WarpDeploymentSuccess.tsx Added modal and UI logic for PR creation flow in deployment success screen, with new state and hooks.
src/features/deployment/utils.ts, src/utils/object.ts, src/utils/string.ts, src/utils/zod.ts New utility functions for config filename generation, config sorting, object key sorting, string normalization, and Zod error handling.
src/consts/config.server.ts, src/types/api.ts, src/types/createPr.ts New config schema, API response types, and PR/signature validation schemas and types.
src/features/deployment/DeploymentDetailsModal.tsx Now sorts deployment result data before rendering using new utility.
src/components/toast/useToastError.tsx Error message extraction improved to check nested error properties.
package.json Added dependencies: @octokit/rest and human-id.
eslint.config.mjs Minor: camelcase rule key changed from string to identifier.
next.config.js Style and formatting improvements, no functional changes.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant WarpDeploymentSuccess
    participant CreateRegistryPrModal
    participant useCreateWarpRoutePR
    participant API_create-pr
    participant GitHub

    User->>WarpDeploymentSuccess: Click "Create PR" button
    WarpDeploymentSuccess->>CreateRegistryPrModal: Open modal
    CreateRegistryPrModal->>useCreateWarpRoutePR: Submit GitHub username/org
    useCreateWarpRoutePR->>API_create-pr: POST /api/create-pr (with configs & signature)
    API_create-pr->>GitHub: Create branch, commit files, open PR
    GitHub-->>API_create-pr: PR URL
    API_create-pr-->>useCreateWarpRoutePR: PR URL response
    useCreateWarpRoutePR-->>CreateRegistryPrModal: Show PR URL
    CreateRegistryPrModal-->>User: Display PR link
Loading

Possibly related PRs

Suggested reviewers

  • paulbalaji
  • cmcewen
  • xeno097

Poem

In the swamp of code, a new path appears,
Warp routes and PRs, no more tears.
GitHub branches sprout, configs sorted neat,
Modals pop open, success tastes sweet.
With signatures checked and errors tamed,
This deployment’s a beauty—never to be shamed!
🐸✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 25d8f0e and 5ae6b0d.

📒 Files selected for processing (1)
  • next.config.js (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • next.config.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: lint
  • GitHub Check: build
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch xaroz/feat/pr-creation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (3)
src/features/deployment/utils.ts (1)

1-56: Consider reusing utils from the registry package

Just like I wouldn't reinvent the wheel when there's a perfectly good boulder to roll, you might want to use the existing utilities from the registry package for generating warp IDs and filenames.

src/features/deployment/github.ts (1)

72-98: Consider using WarpDeploySchema for validation

Like checking if the bridge is sturdy before crossing, you might want to use the WarpDeploySchema from the SDK to validate your deployment config. Keeps things consistent across the codebase.

src/pages/api/create-pr.ts (1)

80-132: PR creation logic looks solid!

Good to see the warpRouteId being used in the changeset message as suggested. The sequential file uploads and PR creation flow is clean and follows GitHub's API patterns well.

🧹 Nitpick comments (6)
src/utils/object.ts (1)

6-11: Shallow sort is grand, but type cast hides a tad of truth

Casting the reduced {} to T swallows type-safety if keys differ. If you fancy stricter smells, start with {} as Partial<T> and return acc as T after the loop. Not critical, just cleaner.

src/utils/zod.ts (1)

21-29: Neat helper – consider explicit return type

Declaring validateStringToZodSchema<T>(...): T | null makes intent clearer to TS and readers alike.

src/types/api.ts (1)

1-4: Consider making the success flag required in ApiError

Well now, having an optional success flag in your error type is like having a swamp that might or might not be murky - you want to be certain about these things. Making it required (success: false) would ensure clearer type discrimination between success and error responses.

 export interface ApiError {
-  success?: false;
+  success: false;
   error: string;
 }
src/features/deployment/CreateRegistryPrModal.tsx (1)

56-56: Simplify with optional chaining

Sometimes the simple path through the swamp is the best one. You can make this check cleaner with optional chaining.

-            {response && response.success ? (
+            {response?.success ? (
src/features/deployment/warp/WarpDeploymentSuccess.tsx (1)

24-211: Consider breaking down this component for better maintainability

This component's gotten bigger than my swamp hut! With all the modal handling, config operations, and PR creation logic, it might be time to extract some of these concerns into custom hooks or smaller components. Makes it easier to test and understand, you know?

src/features/deployment/github.ts (1)

115-129: Multi-protocol support needs implementation

The TODO comment's been sitting there like a log in the swamp. Since you're only supporting EthersV5 right now, users with other wallet types will hit that error faster than you can say "get out of my swamp!"

Would you like me to help implement support for other provider types or create an issue to track this work?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 56549f0 and 0af1496.

⛔ Files ignored due to path filters (2)
  • src/images/icons/folder-code-icon.svg is excluded by !**/*.svg
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (18)
  • eslint.config.mjs (1 hunks)
  • package.json (2 hunks)
  • src/consts/config.server.ts (1 hunks)
  • src/features/deployment/CoinGeckoConfirmationModal.tsx (1 hunks)
  • src/features/deployment/CreateRegistryPrModal.tsx (1 hunks)
  • src/features/deployment/DeploymentDetailsModal.tsx (3 hunks)
  • src/features/deployment/github.ts (1 hunks)
  • src/features/deployment/utils.ts (2 hunks)
  • src/features/deployment/warp/WarpDeploymentSuccess.tsx (3 hunks)
  • src/flows/LandingCard.tsx (1 hunks)
  • src/libs/github.ts (1 hunks)
  • src/pages/api/create-pr.ts (1 hunks)
  • src/types/api.ts (1 hunks)
  • src/types/createPr.ts (1 hunks)
  • src/utils/api.ts (1 hunks)
  • src/utils/object.ts (1 hunks)
  • src/utils/string.ts (1 hunks)
  • src/utils/zod.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (7)
src/utils/object.ts (2)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/utils/object.ts:3-12
Timestamp: 2025-07-03T15:57:43.709Z
Learning: In the sortObjByKeys function in src/utils/object.ts, the user confirmed that for their PR's purpose (creating deterministic branch names via hashing), they only need to sort top-level keys and do not require recursive sorting of nested objects.
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
src/features/deployment/DeploymentDetailsModal.tsx (1)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
src/features/deployment/utils.ts (2)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/utils/object.ts:3-12
Timestamp: 2025-07-03T15:57:43.709Z
Learning: In the sortObjByKeys function in src/utils/object.ts, the user confirmed that for their PR's purpose (creating deterministic branch names via hashing), they only need to sort top-level keys and do not require recursive sorting of nested objects.
src/features/deployment/warp/WarpDeploymentSuccess.tsx (1)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
src/features/deployment/github.ts (1)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
src/pages/api/create-pr.ts (1)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
src/types/createPr.ts (2)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/utils/object.ts:3-12
Timestamp: 2025-07-03T15:57:43.709Z
Learning: In the sortObjByKeys function in src/utils/object.ts, the user confirmed that for their PR's purpose (creating deterministic branch names via hashing), they only need to sort top-level keys and do not require recursive sorting of nested objects.
🧬 Code Graph Analysis (6)
src/utils/api.ts (1)
src/types/api.ts (1)
  • ApiResponseBody (11-11)
src/libs/github.ts (1)
src/consts/config.server.ts (2)
  • ServerConfigSchema (3-28)
  • serverConfig (39-46)
src/features/deployment/DeploymentDetailsModal.tsx (1)
src/features/deployment/utils.ts (1)
  • sortWarpCoreConfig (37-56)
src/features/deployment/CreateRegistryPrModal.tsx (3)
src/components/text/A.tsx (1)
  • A (5-7)
src/utils/string.ts (1)
  • normalizeEmptyStrings (1-8)
src/utils/zod.ts (1)
  • zodErrorToFormikErrors (10-19)
src/features/deployment/warp/WarpDeploymentSuccess.tsx (6)
src/features/deployment/hooks.ts (3)
  • useWarpDeploymentConfig (14-18)
  • useDeploymentHistory (26-39)
  • useLatestDeployment (41-44)
src/features/deployment/github.ts (1)
  • useCreateWarpRoutePR (29-70)
src/features/deployment/warp/utils.ts (1)
  • isSyntheticTokenType (23-25)
src/features/deployment/utils.ts (3)
  • tryCopyConfig (6-12)
  • sortWarpCoreConfig (37-56)
  • getConfigsFilename (28-35)
src/components/text/A.tsx (1)
  • A (5-7)
src/features/deployment/CreateRegistryPrModal.tsx (1)
  • CreateRegistryPrModal (13-79)
src/features/deployment/github.ts (4)
src/features/deployment/hooks.ts (1)
  • useLatestDeployment (41-44)
src/features/deployment/warp/utils.ts (1)
  • isSyntheticTokenType (23-25)
src/features/deployment/utils.ts (2)
  • getConfigsFilename (28-35)
  • sortWarpCoreConfig (37-56)
src/utils/string.ts (1)
  • normalizeEmptyStrings (1-8)
🪛 Biome (1.9.4)
src/features/deployment/CreateRegistryPrModal.tsx

[error] 56-56: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: install
🔇 Additional comments (18)
eslint.config.mjs (1)

58-60: Key looks good without the fancy dress

Dropping the quotes around camelcase still gives ESLint the same tasty key, so no worries in the swamp. Carry on.

package.json (2)

27-27: New Octokit dependency – double-check runtime & bundle impact 🪓

@octokit/rest@^22 shifted to pure ESM; if any serverless edge or older Node runtimes are involved, imports may break. Give it a quick spin locally and peek at the bundle to be safe, lad.


48-48: human-id addition looks fine

Tiny util, no complaints from the swamp.

src/flows/LandingCard.tsx (1)

42-44: Jumping straight to success page – intentional?

Switching from WarpForm to WarpSuccess skips configuration. If that’s by design, all good; if accidental, users will hit an empty swamp. Give it a quick whirl.

src/features/deployment/CoinGeckoConfirmationModal.tsx (1)

18-24: Well, this looks good to me!

The addition of the close prop is clean and follows the expected pattern. Proper typing and consistent with how other modal components handle closing.

src/utils/api.ts (1)

4-12: This utility function is solid!

Nice clean implementation for standardizing API responses. The conditional logic properly handles both cases - sending JSON when there's a body, or just ending with status code when there isn't. The generic typing ensures type safety throughout.

src/utils/string.ts (1)

1-8: This utility does what it says on the tin!

The logic for normalizing empty strings to undefined is sound. The Object.entries/fromEntries pattern works well here, and the trim() check catches whitespace-only strings nicely. The type assertion is reasonable since you're preserving the object structure.

src/features/deployment/DeploymentDetailsModal.tsx (2)

10-10: Good move extracting this into a utility!

Importing the sortWarpCoreConfig utility promotes code reuse and consistency across deployment components.


27-27: Clean application of the sorting utility.

The sorted result ensures consistent display of deployment data, which is especially important for the CollapsibleData component below.

src/libs/github.ts (1)

7-16: This caching approach makes sense for server-side usage.

The logic is sound - validate the config once, cache the authenticated client, and reuse it. The safeParse validation ensures the client is only created with valid configuration.

One thing to keep in mind: the cached client won't refresh if server config changes during runtime, but that's probably fine for most deployment scenarios where config is static.

src/types/createPr.ts (4)

4-20: Well-crafted GitHub identity validation!

This regex pattern properly enforces GitHub's username rules - no leading/trailing hyphens, no double hyphens, and the 39-character limit. Like layers in an onion, every detail matters here.


22-31: Solid file schema definition!

The validation ensures both path and content are non-empty, and the descriptive messages help with debugging. Clean and simple, just the way it should be.


32-47: Clean schema composition!

Nice use of zod's merge() to combine schemas. The warpRouteId validation and type inference are spot on.


48-60: Proper signature verification structure!

All the essential fields for signature verification are here with appropriate validation. The separation of concerns between PR data and verification is well done.

src/pages/api/create-pr.ts (4)

29-51: Good security practices in the handler setup!

Smart move with the environment-specific error messages - keeps internal details hidden in production while helping developers debug locally.


52-79: Well-structured validation flow!

The sequential validation of body, signature, and branch name keeps things organized. The duplicate PR prevention is a nice touch.


134-169: Thorough request validation!

The nested validation of deploy and warp configs ensures data integrity at multiple levels. Like peeling back the layers, each validation step adds confidence.


216-276: Well-implemented utility functions!

The changeset generation is clean, and I see the double sorting in getBranchName is intentional for deterministic hashing - makes sense for consistent branch naming. The branch existence check handles errors gracefully.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/pages/api/create-pr.ts (1)

182-185: Heads up - there's a copy-paste situation here.

Line 184 is checking the address again instead of validating the signature format. Based on the past review comments, I see this was mentioned before and you indicated it was intentional, but it still looks odd.

If this extra address check is really needed, maybe add a comment explaining why? Otherwise it just looks like a mistake waiting to happen.

🧹 Nitpick comments (2)
src/components/toast/useToastError.tsx (1)

10-10: Good improvement, but let's make it even more robust.

Nice touch extracting the nested error message first - makes sense for the new PR creation flow where errors might come wrapped. The fallback logic is solid.

Consider making it a bit more defensive though:

-const errorMsg = error?.error?.message || errorToString(error, errorLength);
+const errorMsg = error?.error?.message || error?.message || errorToString(error, errorLength);

This way we catch both deeply nested errors and simple message properties before falling back to the full error string conversion.

src/pages/api/create-pr.ts (1)

74-78: Good branch collision prevention.

Smart check to avoid creating duplicate PRs with the same config. The error message could be a bit more helpful though - maybe suggest what the user should do next?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cda7250 and add4bfd.

📒 Files selected for processing (3)
  • src/components/toast/useToastError.tsx (1 hunks)
  • src/features/deployment/github.ts (1 hunks)
  • src/pages/api/create-pr.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/features/deployment/github.ts
🧰 Additional context used
🧠 Learnings (1)
src/pages/api/create-pr.ts (2)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/utils/object.ts:3-12
Timestamp: 2025-07-03T15:57:43.709Z
Learning: In the sortObjByKeys function in src/utils/object.ts, the user confirmed that for their PR's purpose (creating deterministic branch names via hashing), they only need to sort top-level keys and do not require recursive sorting of nested objects.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: lint
  • GitHub Check: build
🔇 Additional comments (9)
src/pages/api/create-pr.ts (9)

32-32: Solid HTTP method guard.

Clean and straightforward - catches the wrong method early and returns the right status code. No complaints here.


41-49: Smart environment-aware error handling.

Good thinking here - showing detailed error messages in dev but keeping things vague in production. Security-conscious approach that'll help with debugging without leaking internals.


53-58: Well-structured validation flow.

Nice pattern here - validate the request body first, then the signature. Early returns keep the happy path clean and readable.


140-167: Solid request validation structure.

Clean separation of concerns - parse the main body, then validate each config file separately. The type safety with the return union is a nice touch.


198-212: Thoughtful timestamp validation logic.

Good security practice here - preventing replay attacks with the timestamp check. The 2-minute window seems reasonable for user interactions.

The string parsing approach is straightforward, though you might want to be more specific about the expected message format in case it changes later.


217-229: Neat changeset generation.

Clean implementation following the changesets pattern. The human-readable ID generation is a nice touch for branch names.


236-237: Smart deterministic hashing approach.

From the retrieved learnings, I understand this double sorting is intentional for consistent branch naming. Good thinking - ensures the same config always generates the same branch name regardless of key order.


242-252: Robust hash generation with error handling.

The keccak256 approach for branch naming is solid - gives you consistent, collision-resistant identifiers. Good defensive programming with the try-catch too.


255-277: Clean branch existence checking.

Smart use of the GitHub API's 404 behavior to check if a branch exists. The error handling distinguishes between "branch doesn't exist" (good) and "something went wrong" (bad).

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
src/pages/api/create-pr.ts (1)

127-129: That catch block's still being a bit stingy with the details, mate.

The error handling here only returns the message, but GitHub API errors often pack more useful context in other properties. You'll want the full picture when things go sideways.

  } catch (err: any) {
+   console.error('PR creation failed:', err);
    return sendJsonResponse(res, 500, { error: err.message });
  }
🧹 Nitpick comments (1)
src/pages/api/create-pr.ts (1)

96-96: Consider including more context in the commit message.

The current message is pretty generic. Adding the warp route ID or other identifying details would make the commit history more meaningful.

-    const changesetFile = writeChangeset(`Add ${warpRouteId} warp route deploy artifacts`);
+    const changesetFile = writeChangeset(`Add ${warpRouteId} warp route deploy artifacts`);

Actually, this looks fine as-is since it includes the warp route ID. The commit messages in the file upload loop could be more descriptive though:

-        message: `feat: add ${file.path}`,
+        message: `feat: add ${warpRouteId} ${file.path}`,
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f40f613 and 25d8f0e.

📒 Files selected for processing (2)
  • next.config.js (4 hunks)
  • src/pages/api/create-pr.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • next.config.js
🧰 Additional context used
🧠 Learnings (1)
src/pages/api/create-pr.ts (2)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/utils/object.ts:3-12
Timestamp: 2025-07-03T15:57:43.709Z
Learning: In the sortObjByKeys function in src/utils/object.ts, the user confirmed that for their PR's purpose (creating deterministic branch names via hashing), they only need to sort top-level keys and do not require recursive sorting of nested objects.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: lint
  • GitHub Check: build
🔇 Additional comments (5)
src/pages/api/create-pr.ts (5)

181-182: Clean signature validation - no more double-checking addresses!

Good to see the signature validation is now properly checking both address and signature formats separately. Much better than that earlier mix-up.


195-207: Smart anti-replay protection with timestamp validation.

Nice touch adding the timestamp validation to prevent stale signatures. The 2-minute window strikes a good balance between security and usability.


233-234: Double sorting is doing its job here - no worries.

The intentional double sorting (sortObjByKeys wrapping sortWarpCoreConfig) ensures consistent branch naming through deterministic hashing. This follows the established pattern for this codebase.


258-273: Solid branch existence check with proper error handling.

The logic here is well thought out - using a 404 to determine if a branch doesn't exist, while properly handling other potential errors. Clean and reliable.


214-225: Changeset generation follows the right format.

The changeset structure matches the expected format with proper frontmatter and human-readable IDs. This'll play nice with the changeset workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants